feat(server-utils): Migrate @opentelemetry/instrumentation-graphql to orchestrion#21885
Merged
Conversation
Member
Author
|
bugbot run |
Member
Author
|
bugbot run |
Member
Author
|
bugbot run |
Contributor
size-limit report 📦
|
Member
Author
|
bugbot run |
Member
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 43d4510. Configure here.
chargome
commented
Jul 2, 2026
0eccdf9 to
d2c7ce0
Compare
…to orchestrion Rewrite the graphql integration as a diagnostics-channel subscriber, with orchestrion injecting channels into graphql's compiled `parse`/`validate`/`execute` (v14–v16). Registers `graphqlIntegration` in the channelIntegrations registry and adds a per-lib config file, so the opt-in and `diagnosticsChannelInjectionIntegrations()` pick it up automatically. Span names/attributes/origin mirror the OTel integration (distinct `auto.graphql.orchestrion.graphql` origin); the structural graphql types are shared with the vendored OTel instrumentation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d2c7ce0 to
21acce6
Compare
…phql # Conflicts: # packages/server-utils/src/orchestrion/index.ts
…er conventions develop landed a native diagnostics_channel graphql subscriber (v17) with new span conventions (graphql.document, WEB_SERVER_GRAPHQL_SPAN_OP op, auto.graphql.diagnostic_channel origin). Rework the orchestrion v14–16 subscriber to reuse that module's utils/conventions so spans are identical across versions, and compose the registry entry as native(v17) + orchestrion(v14–16) via extendIntegration so opting into injection instruments every supported version without the OTel patcher (no v17 regression). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…phql # Conflicts: # packages/server-utils/src/orchestrion/channels.ts # packages/server-utils/src/orchestrion/config/index.ts
…phql # Conflicts: # packages/server-utils/src/orchestrion/channels.ts # packages/server-utils/src/orchestrion/config/index.ts # packages/server-utils/src/orchestrion/index.ts
andreiborza
reviewed
Jul 8, 2026
andreiborza
approved these changes
Jul 8, 2026
andreiborza
left a comment
Member
There was a problem hiding this comment.
LGTM, thanks for taking care of this!
This was referenced Jul 8, 2026
Merged
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…c-context logic (#22099) Removes the ioredis unit test that stubbed the SDK's async context strategy to drive spans, which replicated SDK internals in the test layer. Per the review thread in #21885, we rely on the integration tests instead. The orchestrion path is covered by the `redis` (ioredis scenario) and `redis-cache` node integration suites. The `responseHook` and connect-span cases aren't cleanly testable on the orchestrion path (the hook is wired internally for caching) and these suites are Docker-gated, so they're left as a possible follow-up rather than migrated now. Ref: #21885 (comment) Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…nc-context logic (#22101) Removes the mongoose unit test that stubbed the SDK's async context strategy to drive spans, which replicated SDK internals in the test layer. Per the review thread in #21885, we rely on the integration tests instead. The path is covered by the `mongoose-tracing-channel` node integration suite. To avoid losing the useful unit-only assertions, this PR also adds error-status, empty-filter, and cursor-batchSize coverage to that suite. Ref: #21885 (comment) --------- Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…-context logic (#22098) Follow-up to the review discussion in #21885: several `server-utils` diagnostics-channel unit tests stub the SDK's async context strategy (`setAsyncContextStrategy` plus a hand-rolled `installTestAsyncContextStrategy`) to drive spans, replicating SDK internals in the test layer. Per the thread, we're removing these in favor of the integration tests that exercise the real code paths. This PR removes both redis unit tests: - `test/integrations/tracing-channel/redis.test.ts` (orchestrion path, origin `auto.db.orchestrion.redis`) — covered by `suites/tracing/redis` (the suite branches on `isOrchestrionEnabled()`). - `test/redis/redis-dc-subscriber.test.ts` (native diagnostics-channel subscriber, origin `auto.db.redis.diagnostic_channel`) — covered by `suites/tracing/redis-dc`. Ref: #21885 (comment) Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…async-context logic (#22097) Removes the postgres.js unit test that stubbed the SDK's async context strategy to drive spans, which replicated SDK internals in the test layer. Per the review thread in #21885, we rely on the integration tests instead. The orchestrion path is already covered end to end by the `postgresjs` node integration suite. The remaining unit-only assertions checked internal implementation details (not re-creating a span on a re-entrant call, skipping an already-wrapped query, no span without an active parent) rather than the spans a user actually sees, so there was nothing meaningful to move into an integration test. Ref: #21885 (comment) Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…nc-context logic (#22103) Removes the three graphql subscriber unit tests and their shared helper, which stubbed the SDK's async context strategy and a fake document to drive spans, replicating SDK internals in the test layer. Per the review thread in #21885, we rely on the integration tests instead. The path is covered by the `graphql-tracing-channel` node integration suite. To avoid losing the useful unit-only assertions, this PR also adds validation-error status and `ignoreTrivialResolveSpans` coverage to that suite. The subscription-channel case is left as a possible follow-up. Ref: #21885 (comment) --------- Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza
added a commit
that referenced
this pull request
Jul 8, 2026
…ontext logic (#22100) Removes the hapi unit test that stubbed the SDK's async context strategy to drive spans, which replicated SDK internals in the test layer. Per the review thread in #21885, we rely on the integration tests instead. The orchestrion path is covered by the `hapi` node integration suite. `hapi-utils.test.ts` stays (pure metadata formatting, no async-context stubbing). The remaining unit-only assertions checked internal implementation details (setup only wrapping once when called twice, no span without an active span, accepting the ext handler as a tuple or an object) rather than the spans a user actually sees, so there was nothing meaningful to move into an integration test. Ref: #21885 (comment) Co-authored-by: Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates
@opentelemetry/instrumentation-graphqlto orchestrion diagnostics-channel injection.The channel based integration subscibes to
orchestrion:graphql:{parse,validate,execute}and emits the same spans as the OTel integration — with a distinctauto.graphql.orchestrion.graphqlorigin.closes #20752